Repository navigation
Conversation
46a525f to
7466bb9
Compare
|
LGTM One small thing before merging: the main |
|
Oversight thanks for catching. Will fix |
|
Fixed! |
There was a problem hiding this comment.
While doing some extra testing I found a couple of issues, so I'd hold off on merging for now.
I ran the generated projects end-to-end, npm install && npm test on fresh output from generate --standardlint, generate --esm --standardlint and generate-plugin , comparing main against this branch. On main the --standardlint projects fail with standard: command not found (the standard devDependency is gone from the CLI's package.json), so this PR is definitely needed. But on this branch the generated projects still don't pass, for two reasons: one introduced here and one pre-existing.
1. Generated eslint.config.js fails its own lint (blocking)
The lintConfig strings in generate.js L210-L212 have no trailing newline, and ESLint lints its own config file, so npm test in a fresh --standardlint project fails immediately:
eslint.config.js
3:42 error Newline required at end of file but not found @stylistic/eol-last
Same for the ESM variant (eslint.config.mjs, 3:29). The fix is just appending \n to both strings:
template.lintConfig = template.type === 'module'
? 'import neostandard from \'neostandard\'\n\nexport default neostandard()\n'
: '\'use strict\'\n\nmodule.exports = require(\'neostandard\')()\n'2. Pre-existing, but surfaces now that lint passes: node --test picks up test/index.tst.ts
In the plugin template, unit: "node --test" with no arguments uses Node's default patterns, which include **/test/**/*.?(c|m)[jt]s. So the tstyche type-test file gets executed as a unit test and fails with Cannot use import statement outside a module. Not caused by this PR (main has the same script, it was just masked by the earlier standard failure), but since you're already touching the plugin scripts, this would fix it:
unit: 'node --test test/**/*.test.js'Everything else still LGTM, I'll re-approve as soon as these are fixed.
|
I recall testing this more. Apologies for the bugs, let me take a more focused pass and get those cleaned up. |
I meant to have neostandard reconciled with standard by now but that hasn't happened yet so this is the shortest path forward for now. neostandard is the next version of standard and I'm suggesting we swap standard out for it until reconciliation happens. We can switch back or in addition once that happens.
neostandard is an eslint config, so you use it with eslint directly. This just sets up the config and eslint command.
Checklist
npm run test && npm run benchmark --if-presentand the Code of conduct